chore(services): store observed counts in permissionsServer, add LR/LS flags#2929
chore(services): store observed counts in permissionsServer, add LR/LS flags#2929
Conversation
github-actions
bot
added
area/api v1
Codecov Report❌ Patch coverage is ❌ Your project check has failed because the head coverage (35.06%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage.
Additional details and impacted files@@ Coverage Diff @@
## main #2929 +/- ##
===========================================
- Coverage 74.82% 35.06% -39.75%
===========================================
Files 497 430 -67
Lines 60656 54886 -5770
===========================================
- Hits 45382 19243 -26139
- Misses 12115 33724 +21609
+ Partials 3159 1919 -1240 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
barakmich
force-pushed
the
barakmich/ps_counts
branch
from
e95f9b9 to
0bd1e09
Compare
| // This allows tracking which parts of query plans are used most frequently. | ||
| type QueryPlanMetadata struct { | ||
| mu sync.Mutex | ||
| stats map[query.CanonicalKey]query.CountStats // GUARDED_BY(mu) |
There was a problem hiding this comment.
should we put a cap on how much this map can grow?
There was a problem hiding this comment.
It'll grow only as the schema changes. For very large schemas, it may touch ~1000 entries. Rapidly changing large schemas with no overlap could make this grow... but I'm not worried about it. Plus, we're still behind the experimental flag and I think we'll have to come back to it.
barakmich
force-pushed
the
barakmich/ps_counts
branch
from
0bd1e09 to
01dfa21
Compare
barakmich
changed the title
| t.Parallel() | ||
|
|
||
| ctx := NewTestContext(t) | ||
| ctx.Context = t.Context() |
There was a problem hiding this comment.
nit: line 23 of testutil.go is already doing this
2 participants
Description
Add the "lr" and "ls" strings as inputs for
--experimental-query-plan. Adds testing for the gRPC routes along with the existing consistency tests. And in so doing, found another bug: we want to combine the caveat both from the wildcard, and the relationship itself, which may or may not have oneTesting
References